fix variables - #26762

Reviewed the complete diff from merge-base 6798bd63884c3fb363589565f925fd16f94eccbe to head 2b290fc5f5c05f3be1105316f42a4c82df23b04b, including parser generation, frontend compile/execute paths, prepared reuse, planner binding, and tests. Requesting changes for two P1 correctness issues; one P2 performance issue is also recorded inline. No P0 or P3 findings.

Request changes on exact head 2b290fc5f5c05f3be1105316f42a4c82df23b04b.

I independently traced the parser → planner → frontend/status execution paths and found three blocking correctness gaps:

1. SELECT ... INTO @vars validates expression/variable cardinality only after receiving a non-empty batch. select 1 where false into @a, @b therefore succeeds and leaves the variables untouched, while MySQL 8.4 rejects it with error 1222 regardless of row count. I reproduced the silent success through MatrixOne's embedded SQL path; the structural check must be independent of runtime result cardinality.
2. The background execution path installs and fills selectIntoUserVariables, but executeStatusStmtInBack only calls runner.Run and never calls apply. Stored-procedure SQL uses this path, so SELECT ... INTO @var can report success without assigning the variable.
3. Capture keeps only []any and assignment calls SetUserDefinedVar, which hard-codes IsBin=false. Binary-string metadata from the result vector is therefore lost. Later prepared execution (EXECUTE ... USING @v) consults ResolveVariableIsBin, so values assigned by this new syntax can change type/lookup semantics compared with the existing SET path.

There is also an avoidable unhappy-path cost: the collector detects a second row but does not stop execution, and reports the error only after the entire query has completed. Large inputs continue scanning and transporting rows after the outcome is already known.

Focused parser, planner, frontend, and collector tests pass, but they do not cover these execution/metadata/zero-row boundaries. The zero-row counterexample fails against this head exactly because MatrixOne returns nil error.

Reviewed the exact head. Requesting changes for three P1 correctness gaps. First, pkg/frontend/select_into_user_variables.go:61 returns before validating expression and variable arity for a zero-row result, so SELECT 1 WHERE FALSE INTO @A,@b silently succeeds. Validate structural arity independently of runtime batches. Second, line 91 stores only the extracted value and loses the source vector binary flag; preserve per-column IsBin metadata and use the binary-aware setter because EXECUTE USING depends on it. Third, the background and stored-procedure path installs the collector but executeStatusStmtInBack never calls apply, so SELECT INTO reports success without assigning variables. Also return the too-many-rows error as soon as rowCount exceeds one instead of scanning the remaining result.

Reviewed the complete diff from merge-base a4b0ce286d182c24efd5a349620ae37016262301 to exact head ce302fc95481492fd2714acb65cdc34251ffc42c, including parser generation, SELECT-INTO normal/background/prepared execution, user-variable type binding and value reconstruction, diagnostics, lifecycle/Q1-Q3 paths, and tests. The author explicitly replied to the previous P2 comments in the PR conversation, and those prior findings are addressed on this head.

This pass confirms three new P1 correctness defects: array-valued user variables can be reconstructed with invalid raw bytes and panic, TIMESTAMP user variables can shift across session/process time zones, and INTO clauses nested in UNION/parenthesized query trees can be silently dropped. No P0, P2, or P3 findings. Requesting changes because P1 blockers remain.

All 26 GitHub checks are terminal with no failures, and git diff --check is clean. A PR-specific targeted-test worktree could not be created because this isolated repository exposes .git/worktrees read-only; no code or worktree files were modified.

Re-reviewed exact head ce302fc after the successful CI rollup. The follow-up closes the earlier blockers: zero-row arity is validated before execution, second rows fail during capture, binary/type metadata is retained, and frontend/background execution both apply the collected variables.

Reviewed the complete diff from merge-base ff25f27397ab7f1a2952eec98497bfb58419e2c2 to exact head 9ee65d20eca6fefbd91dfb47a1df1a9288d854ea, including grammar/generated parser changes, AST propagation, frontend normal/background/prepared execution, typed user-variable binding/evaluation, diagnostics, and tests. Two P1 blockers remain, plus two P2 compatibility gaps, so I am requesting changes.

Findings are recorded inline:

• P1: reused array/vector variable executors replace valid element bytes with display-text bytes and can panic on a later batch.
• P1: INTO clauses in scalar/derived/CTE/EXISTS subqueries are still accepted and silently dropped.
• P2: mixed pre-FROM/terminal user-variable and OUTFILE clauses can create an AST with both actions, after which export is silently skipped.
• P2: warning diagnostics are not reflected in the OK warning count, and SHOW ERRORS is no longer filtered to errors.

The previous review on ce302fc95481492fd2714acb65cdc34251ffc42c contained only P1 findings, so the author-response rule for prior P2/P3 findings does not affect this event. Those three prior threads are marked resolved but contain no author replies; the array and nested-INTO follow-ups remain incomplete as described below.

Validation: all 26 GitHub checks are terminal with no failures, git diff --check is clean, and GOWORK=off go test -mod=readonly -count=1 -timeout=120s ./pkg/common/moerr/... passed in the PR-specific worktree. The controlled parser/util/colexec test command could not start because that isolated worktree does not contain cgo/libmo.dylib; I am not treating it as passing evidence.

Deep-reviewed exact head 9ee65d20eca6fefbd91dfb47a1df1a9288d854ea across grammar/generated parser, AST ownership, planner binding/cache, frontend normal/background/PERFORM execution, colexec reuse, diagnostics, unhappy paths, and the latest-main merge tree. Four P1 correctness blockers remain:

1. Array/vector user-variable reuse is still unsafe. The first Eval encodes a typed slice with arrayUserVariableValueToBytes; the reuse path treats every non-JSON varlen type as text and calls SetConstBytes(fmt.Sprintf("%v", value)). I independently reproduced a second-evaluation panic for []float32{1,2,3}: decode slice that is not a multiple of element size. Array-related types need the same typed reconstruction on every value refresh, not the generic varlen text path.

2. Nested SELECT ... INTO @v still has no execution owner. SelectIntoVariablesForTopLevel traverses only Select/SelectClause/ParenSelect/UnionClause and does not inspect scalar, derived-table, CTE, or EXISTS subqueries. All of SELECT (SELECT 1 INTO @x), SELECT * FROM (SELECT 1 INTO @x) d, WITH d AS (SELECT 1 INTO @x) SELECT * FROM d, and SELECT EXISTS(SELECT 1 INTO @x) are accepted by the parser in a focused counterexample test, but the outer statement has no IntoVars, so assignment is silently dropped. Either reject INTO outside the supported top-level/final query block or propagate it to a single well-defined execution owner.

3. PERFORM SELECT 1 INTO @x is accepted, but executeStatusStmt handles st.IsPerform before the IntoVars branch and returns after runner finalization without calling the collector apply path. This is another successful silent no-op. Reject this combination consistently with unsupported PERFORM export forms, or define and implement its assignment semantics.

4. User-variable type binding is stale across the transparent session plan cache. A normal SELECT @v + 0 is cacheable, SET @v = ... is deliberately exempted from ses.cleanCache(), and the cached plan retains the VarRef type resolved from the old assignment. A sequence such as integer assignment → cached select → decimal assignment → same select therefore reuses the old integer expr.typ; runtime reconstruction then parses the current decimal through the stale integer vector type, producing an error or wrong coercion. Ad-hoc SQL must be rebound or the cache entry invalidated when a user-variable assignment can change its type.

Two compatibility gaps are also confirmed and should be closed in this update: mixed pre-FROM user-variable INTO plus terminal OUTFILE is accepted and the frontend silently chooses assignment over export; Warning 1329 is not reflected in the OK-packet warning count, while the shared SHOW handler lets SHOW ERRORS include Warning rows.

Previous blockers for zero-row arity, second-row early failure, binary/type retention, background apply, TIMESTAMP timezone, and final UNION/parenthesized INTO propagation are fixed on this head.

Validation: all 26 GitHub checks are terminal without failure; exact-head full frontend and colexec race tests passed; focused select-into race tests passed at -count=13; standard moerr, frontend, colexec, parser, planner, and sql/util packages passed; git diff --check is clean; and the reviewed tree merges cleanly with the latest local main. Temporary counterexample tests were reverted.